fix(boolean-contains): treat an equal two-vertex line as contained - #3080
Merged
mfedderly merged 1 commit intoJun 25, 2026
Merged
Conversation
mfedderly
approved these changes
Jun 25, 2026
mfedderly
left a comment
mfedderly
left a comment
Collaborator
There was a problem hiding this comment.
Thank you!
This also aligns with PostGIS which I've been using as the source of truth for these things:
SELECT ST_Contains(
ST_MakeLine(ST_MakePoint(0, 0), ST_MakePoint(10, 0)),
ST_MakeLine(ST_MakePoint(0, 0), ST_MakePoint(10, 0))
);
st_contains
-------------
t
(1 row)
Collaborator
|
Overall performance hit looks to be ~8%, which is probably acceptable for a correctness change. We can try to optimize this further later. This particular input would be caught by just a straight lineString1 === lineString2 coordinate comparison. Before: After: |
mfedderly
pushed a commit
that referenced
this pull request
Jul 21, 2026
…ths (#3099) * perf(boolean-contains): use native MultiPolygon support in Point check booleanPointInPolygon iterates MultiPolygon members natively, so a single call replaces one wrapped Polygon call per member - paying the per-call setup (validation, getCoord, getGeom) once instead of N times and avoiding the intermediate Polygon object allocations. Identical semantics: a point strictly inside any member polygon. * perf(boolean-contains): reduce point-on-line probes in MultiPoint check isMultiPointOnLine ran the interior probe (ignoreEndVertices: true) unconditionally for every point, even after an interior point had already been found, and ran it before the membership check - so points not on the line paid for both scans before failing. Check membership first (early exit), and only probe for an interior point until one is found. Identical semantics. * perf(boolean-contains): reduce point-on-line probes in LineString check isLineOnLine ran the interior probe (ignoreEndVertices: true) unconditionally for every vertex, even after an interior point had already been found, ran it before the membership check, and wrapped every coordinate in a fresh Point object although booleanPointOnLine accepts raw positions. Check membership first (early exit), only probe for an interior point (vertex, then segment midpoint) until one is found, and pass raw coordinates. Identical semantics, including the segment-midpoint interior detection from #3080. * perf(boolean-contains): avoid redundant bbox computation in MultiPolygon polygon checks isPolygonInMultiPolygon and isMultiPolygonInMultiPolygon called isPolyInPoly once per member polygon, and each call recomputed the bbox of both geometries - so the candidate polygon's bbox was recomputed N times for an N-member MultiPolygon. Let isPolyInPoly accept an optional precomputed bbox for the candidate polygon and compute it once per candidate in the MultiPolygon loops. Identical semantics. An additional whole-MultiPolygon bbox prefilter was tried: it sped up the fully-outside case by about 50%, but regressed the overlapping-bbox cases by ~15%, since `calcBbox` over all members is pure added cost whenever the check passes.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
booleanContainsdocuments the dualitycontains(a, b) === within(b, a), and as a containment relation it is reflexive: a geometry contains an equal copy of itself. Both properties break for the simplest LineString case.Two identical 2-vertex lines, e.g.
[[0,0],[10,0]]and[[0,0],[10,0]], returncontains = false.The cause is in
isLineOnLine, which only records an interior overlap when a vertex oflineString2is strictly interior tolineString1(ignoreEndVertices: true). For two equal 2-vertex lines, both of line2's vertices are endpoints, so neither is strictly interior, and the function reports no interior overlap even though the lines are identical.This is a vertex-counting artifact rather than a geometric one: inserting a redundant collinear midpoint into line2 (
[[0,0],[5,0],[10,0]]) flips the answer totrue, because the added vertex is now strictly interior. The same line, described with one more vertex, should not change containment.Fix
Keep the existing vertex check, and additionally probe each segment midpoint of
lineString2. If a segment's midpoint is strictly interior tolineString1, the segment shares interior withlineString1, so an interior overlap exists even when no vertex oflineString2is strictly interior. This is the minimal probe that handles the equal-2-vertex case without changing any other result.Tests
Fixture added:
test/true/LineString/LineString/LineContainsEqualTwoVertexLine.geojson(two identical 2-vertex lines must contain each other). It fails on the current source and passes after the fix.boolean-contains: 40/40.boolean-within(duality regression): 40/40.